Skip to content

Conversation

@haoxins
Copy link
Contributor

@haoxins haoxins commented Sep 19, 2015

No description provided.

src/table.js Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't upgradeElement need something like 'MaterialTable'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes. It's MaterialDataTable

@phated
Copy link

phated commented Sep 21, 2015

Maybe this should have separate elements, like suggested in the Tabs PR. It doesn't do anything special with the insides of the table, so maybe we can just inject children and the end user can put normal table contents in it. That way you just replace <table> with <Table> and get material styling on it. Thoughts?

@RangerMauve
Copy link

Material-ui originally went with a Table component that just had like over nine thousand configuration options for their initial version. However they split that up into a bunch of components that represent parts of a table (TableBody, TableRow, etc).

@haoxins
Copy link
Contributor Author

haoxins commented Sep 22, 2015

That way you just replace <table> with <Table> and get material styling on it.

There are some optional classes on table data cells, such as mdl-data-table__cell--non-numeric.
And the user could pass ReactElement as columns and rows also.

@phated
Copy link

phated commented Sep 22, 2015

@coderhaoxin I think we should have separate components for rows, columns, etc

@haoxins
Copy link
Contributor Author

haoxins commented Sep 23, 2015

@phated

have separate components for rows, columns

done

@phated
Copy link

phated commented Oct 2, 2015

I think the mapping needs to go away. A component library should just accept children, not custom DSLs of arrays of objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants